Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] fix SQL Server command generation #35317

Merged
merged 2 commits into from Nov 23, 2020
Merged

[8.x] fix SQL Server command generation #35317

merged 2 commits into from Nov 23, 2020

Conversation

rodrigopedra
Copy link
Contributor

@rodrigopedra rodrigopedra commented Nov 22, 2020

The new php artisan db command introduced in PR #35304 adds a very nice feature that allows the developer to easily connect to the DBMS CLI using the connection parameters already configured in the application.

But as noted by that PR author, he didn't have SQL Server available, then he couldn't test the generated command against it.

I tested locally the new command and the command-line generated by the original PR fails when executed. This PR fixes the command for SQL Server.

Sample command generated before this PR:

'sqlcmd' '-d database' '-U sa' '-P Password123456' '-S tcp:127.0.0.1,1433'

Note command options and their values are quoted together.

Sample command generated after this PR:

'sqlcmd' '-d' 'database' '-U' 'sa' '-P' 'Password123456' '-S' 'tcp:127.0.0.1,1433'

@paras-malhotra
Copy link
Contributor

Thank you @rodrigopedra 👍

@rodrigopedra
Copy link
Contributor Author

You're welcome! @paras-malhotra

Thank you for the nice addition, very handy!

@taylorotwell taylorotwell merged commit 056a033 into laravel:8.x Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants